@twop I would be interested on your opinions on this design given your work in #198

@twop I would be interested on your opinions on this design given your work in #198

@nicoburns I love the idea! I think it solves some paint points of the sketch I made in #198, but it is spiritually the same what I had in mind. I think your implementation is more elegant :)

I'm not sure about the exact API ergonomics, for example

 pub fn compute_layout_with_context(
        &mut self,
        node: NodeId,
        available_space: Size<AvailableSpace>,
        context: Measure::Context,
    ) -> Result<(), TaffyError> {
        self.context = Some(context);
        let result = compute_layout(self, node, available_space);
        self.context = None;
        result
    }

feels intuitively a bit off, specifically that pattern.

   self.context = Some(context);
   self.context = None;

But I don't know on top of my head how it can be structured a bit better.

Overall, I think that direction is a big win in my head for the library (as a lib user). Nice job 👍!!

One other interesting detail/observation: Context needs to be mut for measure call (for example for caching, thus changing internal state), so compute_layout_with_context takes ownership of Context to potentially mutate it internally. I wonder if it is worth giving a shot passing &mut Context instead, it can be potentially more ergonomic for implementers.

Maybe I'm missing something, but it seems that using the API properly might involve some Rc<RefCell> voodoo, at least in some cases, thus &mut Context with potentially some lifetime tracking might provide better dev ergonomics

@nicoburns I'm sorry that I'm "climbing into someone else's monastery with my charter" (I'm a C++ programmer, and I only "read" Rust, and don't "write"), but doesn't instantiating a Taffy template/generic with different types generate twice as much code? For example:

let mut taffy1: Taffy<MeasureFunc<TextLayoutData>> = Taffy::new(); // Instantiation for one type
let mut taffy2: Taffy<MeasureFunc<SomeOtherData>> = Taffy::new();  // Instantiation for another type

Two different instances produces 2 unique Taffy specializations & double code generation (if in Rust it works same as in C++).
I know, that its unexpected/unusal code example (2 separated intances of nodes tree), but even if we have only single instance for application, and we need extra data during measuring widget size - can we avoid the template parameter for the whole Taffy tree structure (since it may be various types in different cases)?

For example, in C++, not too-safe, but good-old common-pattern for passing any context - using non-owning void pointer (type erasurement from known type), and then back - casting into known type when it's needed. For example (C++-like pseudocode):

class SomeSpeciallyMeasuredWidget
{
    taffy::NodeId _id;
    taffy::Style _style;
    
    SomeUsefulDataForMeasurement _meas_data;
    
    SomeSpeciallyMeasuredWidget (Widget* parent)
         : Widget(parent)
    {
         taffy::Taffy* taffy_tree = this->getRootTree();
         // `this` is mutable pointer to self, here is it passed as `void*`
         _id = taffy_tree->new_leaf_with_measure_and_context(_style, this, 
             [](const Size<Option<float>>& known_dimensions, const Size<taffy::AvailableSpace>& available_space, void* context) -> taffy::Size<float>
             {
                 SomeSpeciallyMeasuredWidget* self = static_cast<SomeSpeciallyMeasuredWidget*>(context); // from void* into known type
                 return my_measurement_computation(known_dimensions, available_space, self->_meas_data);
             }
         );
    }
};

Maybe somehow we can achieve the same in Rust?

@inobelar I think your intuition is correct, but I'm not sure what is the impact of the issue in practice.

E.g. How many different Measure implementations any given project uses? Judging from personal subjective experience, I don't see a common usage pattern where that is the case 🤷‍♀️

@inobelar You're definitely correct that this will generate two copies of everything. And this definitely seems non-ideal given that it's only a very small part of the code that would actually need to change.

The Rust equivalent of void * in this case would probably be &mut dyn Any: that can be downcast to &mut dyn T if you know the correct T. I'm a little reluctant to make that change though, as it would require all measure function implementations (written by the user) to manually downcast the value.

The other way to avoid duplicating everything would be to make the algorithms take &mut dyn LayoutTree rather than &mut impl LayoutTree. But I'm also reluctant to do that as I suspect that might cause performance issues.

The expected way of using Taffy would be:

struct TextLayoutData { ... }
struct SomeOtherData { ... }

enum CombinedData {
     Text(TextLayoutData),
     Other(SomeOtherData),
}

let mut taffy: Taffy<MeasureFunc<CombinedData>> = Taffy::new();

which also avoids this problem. And I agree with @twop that it would be uncommon to need multiple Taffy instances with different Context parameters.

I'm building a integration to use taffy for egui for more complex layouts. I based my work on this PR since I hoped it would solve the lifetime issue I was facing, but in the end I had to use some unsafe code to make things work.

    let mut taffy = TaffyPass::new(
        ui,
        Id::new("flexible"),
        Style {
            display: Display::Flex,
            ..Default::default()
        },
    );

    taffy.add(
        Id::new("child_2"),
        Style {
            flex_grow: 1.0,
            ..Default::default()
        },
        Layout::centered_and_justified(Direction::TopDown),
        |ui| {
            let _ = ui.button("Button 1");
        },
    );

    taffy.add(
        Id::new("child_3"),
        Style {
            flex_grow: 1.0,
            ..Default::default()
        },
        Layout::centered_and_justified(Direction::TopDown),
        |ui| {
            let _ = ui.button("Button 2");
        },
    );
    taffy.show();

Since egui is immediate mode and I'm creating this as a library to be used in combination with egui's layout capabilities, my use of taffy is probably a bit different than most others. But I think what I'm trying to do should be possible with safe rust.

My problem is: The last parameter to the add function is a FnMut closure. I want to pass this as a &mut reference to the measure function and then after the layout was calculated, I want to call it again to create the actual ui. But since the compute_layout_with_context requires the context to live for the length of the Taffy struct, this is not possible.

Here is a simple example that fails due to a similar lifetime problem:

use taffy::prelude::{Size, Style};
use taffy::tree::MeasureFunc;
use taffy::Taffy;

struct TaffyState<'f> {
    taffy: Taffy<MeasureFunc<&'f mut str>>,
}

fn main() {
    let mut state = TaffyState {
        taffy: Taffy::new(),
    };

    let node = state
        .taffy
        .new_with_children(Style { ..Style::default() }, &[])
        .unwrap();

    {
        let mut str = "hello".to_string();

        let result = state
            .taffy
            .compute_layout_with_context(node, Size::length(100.0), &mut str)
            .unwrap();
    }
}

For reference, here is the unsafe code I had to write to make things work. I think what I'm doing there should be sound, but I'm not very experienced with rust safety.

Even if this won't be possible for some reason, your PR makes what I'm trying to do much easier, so I really appreciate it!

Hmm… the lifetime being required to outlive the Taffy struct doesn’t sound right! I suspect this is a result of the hack (temporarily storing the context in the Taffy struct for the duration of the layout) that is currently being used to avoid having to pass the context around.

I suspect I’m going to have to update this PR to do things properly, which will likely mean creating a TaffyView struct or similar which can act as an “iterator” over the Taffy tree without actually being the tree. We should be able to store the context in that struct without causing this lifetime issue.

I’m quite busy over the next month, so I’m not sure when I’ll next have time to work on Taffy. But this PR is top of my list, and the change discussed above is the main thing blocking this PR from being merged, so hopefully it shouldn’t be too long :)

@nicoburns , again I apologize for the offtopic, but adding of this new 'generic' parameter also will affect the 'C bindings' #404 and 'Wasm bindings' #394 PRs, or any 'bindings to other languages' in general.

Since, we cannot expose generics through 'C ABI' - we need some 'specialization' for that case. Is it will be: Taffy<MeasureFunc<()>>, or something different?

C and WASM bindings are actually one of the motivations for this PR. It allows them to define their own type to use as the measure function. For C, I am imagining this being something like:

struct CMeasureFunc {
    context: *mut (),
    measure: fn(
        known_dimensions: Size<Option<f32>>,
        available_space: Size<AvailableSpace>,
        context: *mut ()
     ) -> Size<f32>,
}

where *mut () is a Rust representation of a C void * pointer. (there will probably need to be some tweaks to the types of the known_dimensions and available_space parameters too, in order to avoid using data-containing enums).

For WASM, there would be some type that wraps a handle to a JavaScript function. Notably such functions are !Send and therefore won't work well with the regular MeaureFunction.

@alice-i-cecile Release notes added :)